Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: suppress getSession warning whenever _saveSession is called #895

Merged
merged 1 commit into from
May 1, 2024

Conversation

kangmingtay
Copy link
Member

@kangmingtay kangmingtay commented May 1, 2024

What kind of change does this PR introduce?

@kangmingtay kangmingtay merged commit 59ec9af into master May 1, 2024
4 checks passed
@kangmingtay kangmingtay deleted the km/suppress-get-session-warning branch May 1, 2024 12:04
@dagassa
Copy link

dagassa commented May 2, 2024

🙏

kangmingtay pushed a commit that referenced this pull request May 3, 2024
🤖 I have created a release *beep* *boop*
---


##
[2.64.2](v2.64.1...v2.64.2)
(2024-05-03)


### Bug Fixes

* signOut should ignore 403s
([#894](#894))
([eeb77ce](eeb77ce))
* suppress getSession warning whenever _saveSession is called
([#895](#895))
([59ec9af](59ec9af))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@larbish
Copy link

larbish commented May 13, 2024

Hey @kangmingtay, maintainer of the nuxt/supabase module here.

We have a PR to migrate on the @supabase/ssr package and we're still experiencing this issue with the latest released version.

I've removed all occurrences of getSession() in the module and I still have the warning.

Any help on this would be appreciate 🙏 I can't merge and release this PR until I get rid of this warning.

@ekendra-nz
Copy link

Still get :

Using the user object as returned from supabase.auth.getSession() or from some supabase.auth.onAuthStateChange() events could be insecure! This value comes directly from the storage medium (usually cookies on the server) and many not be authentic. Use supabase.auth.getUser() instead which authenticates the data by contacting the Supabase Auth server.

with

		"@supabase/supabase-js": "^2.44.4",
		"@supabase/ssr": "^0.4.1"
		

@AdrienLF
Copy link

AdrienLF commented Aug 4, 2024

Same here with

		"@supabase/ssr": "^0.4.0",
		"@supabase/supabase-js": "^2.45.0"

@marcusklausen
Copy link

marcusklausen commented Sep 6, 2024

Any reason there's not just a prop we can pass to getSession() to suppress the warning manually? This safeguarding is very overboard I think. It is already perfectly clear that getSession is not safe for authorization on it's own.

I think it's a very common thing to use getSession in nextjs middleware. Having to do a roundtrip to supabase servers using getUser on each request on an edge network is just bonkers, so getSession makes sense for the routing. And using getSession in middleware causes an unbelievable amount of warnings in the log, to the point where debugging other stuff becomes really cumbersome.

@greendra8
Copy link

I've gone banner blind to this log I've seen it so much. Please fix!

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants